Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add x_explicit_api_mode to remaining kotlinc_options #607

Merged
merged 1 commit into from
Nov 9, 2021

Conversation

pettermahlen
Copy link
Contributor

See https://github.com/Kotlin/KEEP/blob/master/proposals/explicit-api-mode.md for more about the switch itself.

I'm not sure how to test this - the best test would be to have some test code with default access modifiers and explicit-api=strict set, and then run a compilation and verify that it fails. That seems complex, and like it would require a fair amount of tooling, which I'm not sure if it's available. Please advice if so.

@Bencodes
Copy link
Collaborator

Support for this was already added here #576. Does it not work?

@pettermahlen
Copy link
Contributor Author

No, it's not working, for some reason, although I hadn't even noticed this PR, nor the fact that the options are in fact documented here https://bazelbuild.github.io/rules_kotlin/kotlin.html#kt_kotlinc_options...

I'm new to Bazel, but it looks like the opts.bzl that gets pulled in locally is from /private/var/tmp/_bazel_petter/602bfdc6fd57d9ddfc7aad7b3ee3cb9e/external/io_bazel_rules_kotlin_configured/kotlin/opts.bzl, which doesn't contain the explicit API setting. The version we're pulling in is v1.5.0-beta-4, which AFAICT should contain #576. Is there some configuration option that is wrong, perhaps? (Thinking about the _configured suffix).

@pettermahlen
Copy link
Contributor Author

Hm, the reason I made this change was that my BUILD file does the following load (as per the README):

load("@io_bazel_rules_kotlin//kotlin:core.bzl", "kt_kotlinc_options")

And AFAICT, core.bzl loads from kotlin/internal, or https://github.com/bazelbuild/rules_kotlin/blob/master/kotlin/internal/opts.bzl, which in turn points to the files I changed in this PR. Assuming I read the code right, I am a complete newbie as mentioned.

There seems to be 8 opts.bzl files in the repo (including one named opts.release.bzl). Is that what's causing the issue?

@Bencodes
Copy link
Collaborator

Bencodes commented Nov 1, 2021

Ah so the solution here is to replicate the existing x_explicit_api_mode option in the newly added opts files that are missing this explicit api mode attribute. Do you want to update your PR to do that?

@pettermahlen pettermahlen changed the title Add x_explicit_api to kotlinc_options Add x_explicit_api_mode to remaining kotlinc_options Nov 1, 2021
@pettermahlen
Copy link
Contributor Author

Done. AFAICT, it was only the 1.5 file that was missing the option.

@Bencodes
Copy link
Collaborator

Bencodes commented Nov 2, 2021

Lgtm but @cgruber or @restingbull will need to merge this

@restingbull restingbull merged commit e11db71 into bazelbuild:master Nov 9, 2021
@pettermahlen pettermahlen deleted the explicit-api branch February 3, 2022 12:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants